fix(ui): drop nodeDepth partitioning so ELK derives lineage layers from edges#29224
Open
chirag-madlani wants to merge 4 commits into
Open
fix(ui): drop nodeDepth partitioning so ELK derives lineage layers from edges#29224chirag-madlani wants to merge 4 commits into
chirag-madlani wants to merge 4 commits into
Conversation
…om edges Pinning each node to its backend nodeDepth via elk.partitioning forced a multi-branch node into the column of its first depth, misaligning its other edges. Let ELK's layered algorithm derive layers from edge topology and use BRANDES_KOEPF node placement for a cleaner, less rigid horizontal flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
Contributor
Contributor
🔴 Playwright Results — 1 failure(s), 11 flaky✅ 4018 passed · ❌ 1 failed · 🟡 11 flaky · ⏭️ 84 skipped
Genuine Failures (failed on all attempts)❌
|
BRANDES_KOEPF aligned a node to one child (top-biased), so a source with children Y and Z sat parallel to Y instead of vertically centered. Switch node placement to NETWORK_SIMPLEX, which assigns cross-axis coordinates by minimizing weighted edge deviation, keeping a source balanced between its children. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NETWORK_SIMPLEX (and bare Brandes-Köpf) resolve a symmetric source's placement to a top-aligned corner because all alignments tie on edge length, so the source did not sit centered between its upstream/downstream branches. Setting elk.layered.nodePlacement.bk.fixedAlignment to BALANCED averages the four extreme alignments into a centroid, restoring vertical centering while keeping edge-aware, straight-segment placement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ulixius9
approved these changes
Jun 22, 2026
Rohit0301
approved these changes
Jun 22, 2026
Code Review ✅ ApprovedRemoves nodeDepth partitioning from the lineage graph to resolve node misalignment and cyclic graph crashes, allowing ELK to derive layers from edge topology. Unit tests successfully updated to reflect the new layout configuration. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Describe your changes:
Fixes #29225
I dropped the
nodeDepth-based ELK partitioning in the lineage graph and let ELK'slayeredalgorithm derive layers from edge topology (withBRANDES_KOEPFnode placement) because pinning each node to its single backendnodeDepthforced a multi-branch node into the column of its first depth while its other edges fanned out to different columns, producing a visually misaligned graph. The result is a cleaner, less rigid horizontal flow, and it also removes the partition-on-cyclic-graph crash source.Type of change:
High-level design:
N/A — small change (3 files): removed the per-node
elk.partitioning.partitionhint ingetELKLayoutedElements, and removedelk.partitioning.activatewhile switchingelk.layered.nodePlacement.strategyfromSIMPLEtoBRANDES_KOEPFinELKUtil. Focus/root centering is unaffected — it is handled separately bycenterNodePosition, not by partitioning.Tests:
Use cases covered
Unit tests
ELKUtil.test.tsto match the new layout options (19/19 pass).Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
ELKUtilunit suite passes.nodeDepthin layout and that focus centering is independent.UI screen recording / screenshots:
Issue:
Screen.Recording.2026-06-22.at.5.50.17.PM.mov
Fix:
Screen.Recording.2026-06-22.at.5.50.57.PM.mov
Checklist:
Greptile Summary
This PR fixes visual misalignment in the lineage graph by dropping ELK's
partitioningmode (which pinned each node to its backend-providednodeDepthcolumn) and switching to theBRANDES_KOEPFnode placement strategy withBALANCEDalignment, letting ELK derive layer assignments from actual edge topology.ELKUtil.ts: Removeselk.partitioning.activateand replacesSIMPLEnode placement withBRANDES_KOEPF+elk.layered.nodePlacement.bk.fixedAlignment: BALANCED, with clear inline comments explaining each choice.EntityLineageLayoutUtils.ts: Drops the per-nodeelk.partitioning.partitionhint derived fromnode.data?.nodeDepth, removing the crash path on cyclic graphs.ELKUtil.test.ts: All 19 unit tests updated to match the new layout options.Confidence Score: 5/5
Safe to merge — the change removes a layout constraint that caused visual misalignment and cyclic-graph crashes, with no API or data model changes.
The diff is small and surgical: two layout option keys removed, two added, and corresponding test updates. The nodeDepth field on node data is simply ignored now rather than deleted, so no callsites break. The fallback in getELKLayoutedElements already handles ELK errors gracefully. Tests cover the new configuration exhaustively.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Before["Before (partitioning)"] A1[Node data\nnodeDepth=N] -->|per-node layoutOption| B1["elk.partitioning.partition = N"] B1 --> C1["ELK layered\n+ elk.partitioning.activate=true\n+ nodePlacement: SIMPLE"] C1 --> D1["Multi-branch node locked to\nits first backend depth column\n→ visual misalignment / cyclic crash"] end subgraph After["After (topology-driven)"] A2[Node data\nnodeDepth ignored] -->|no partition hint| B2["No per-node layoutOptions"] B2 --> C2["ELK layered\n+ nodePlacement: BRANDES_KOEPF\n+ bk.fixedAlignment: BALANCED"] C2 --> D2["ELK derives layers from\nedge topology → clean layout"] end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD subgraph Before["Before (partitioning)"] A1[Node data\nnodeDepth=N] -->|per-node layoutOption| B1["elk.partitioning.partition = N"] B1 --> C1["ELK layered\n+ elk.partitioning.activate=true\n+ nodePlacement: SIMPLE"] C1 --> D1["Multi-branch node locked to\nits first backend depth column\n→ visual misalignment / cyclic crash"] end subgraph After["After (topology-driven)"] A2[Node data\nnodeDepth ignored] -->|no partition hint| B2["No per-node layoutOptions"] B2 --> C2["ELK layered\n+ nodePlacement: BRANDES_KOEPF\n+ bk.fixedAlignment: BALANCED"] C2 --> D2["ELK derives layers from\nedge topology → clean layout"] endReviews (4): Last reviewed commit: "Merge branch 'main' into fix-lineage-lay..." | Re-trigger Greptile